Skip to content

Cache benchmark results by base commit - #11684

Open
Bill Schnurr (bschnurr) wants to merge 5 commits into
mainfrom
benchmark/cache-base-results
Open

Cache benchmark results by base commit#11684
Bill Schnurr (bschnurr) wants to merge 5 commits into
mainfrom
benchmark/cache-base-results

Conversation

@bschnurr

@bschnurr Bill Schnurr (bschnurr) commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

  • Compare each PR synthetic merge commit against the exact main commit used to create that merge.
  • Cache validated base benchmark results by commit and hosted measurement profile.
  • Keep dated, checked-in main benchmark results with commit metadata for historical charts.
  • Publish results through artifacts, the job summary, and an updated bot comment.
  • Report benchmark corpus or profile changes as successful but not comparable.

What is compared

The candidate is GitHub's synthetic merge commit, which combines the PR's current head with its current main base. The baseline is that same exact main base commit:

main commit B ----------------------------> baseline benchmark
           \
            + PR head H -> synthetic M ---> candidate benchmark

comparison: M (B + PR changes) versus B

This measures the performance impact the PR would have after merging. It does not compare the PR with its initial commit, benchmark the raw PR head alone, or compare against a moving newer main. If the PR head or base changes before trusted reporting validates the revisions, the report stops and /benchmark must be requested again.

Security and behavior

  • /benchmark is restricted to maintainers and reruns an existing unprivileged pull_request workflow.
  • The candidate workflow has read-only permissions and no cache access.
  • Base benchmarking, cache restore/save, and reporting run separately under trusted default-branch workflow_run code.
  • Trusted jobs check out only ${{ github.sha }}; candidate output crosses the boundary only as bounded, provenance-validated JSON.
  • Cache restores require an exact key and validated revision/profile metadata.
  • Newly measured base results can update the dated and latest baseline files only for same-repository PRs whose head has not changed.

Benchmark history

Checked-in results under build/benchmark/baselines/ include the source commit SHA, subject, and timestamp. build/benchmark/benchmark_history.ipynb graphs execution-time and peak-memory changes across those main commits and can export a static dashboard under docs/benchmark-results/.

Dashboard preview

The checked-in history currently contains one hosted main baseline, so these previews show the initial package measurements. Lines and commit-over-commit changes will appear as dated runs accumulate.

Execution time

Pyright execution time benchmark preview

Peak memory

Pyright peak memory benchmark preview

After merge, the full dashboard will be available at https://microsoft.github.io/pyright/benchmark-results/.

Validation

  • python -m unittest discover -s build/benchmark -p "test_*.py" (52 tests)
  • Workflow YAML parsing and Prettier checks
  • CodeQL, including Actions analysis

Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Resolve PR revisions from trusted GitHub metadata before executing benchmark code. Retain and update checked-in baseline results with commit provenance, and add a notebook for benchmark history visualization and dashboard export.
Comment thread .github/workflows/typecheck_benchmark_pr.yml Fixed
Run candidate benchmarks in an unprivileged pull_request workflow and move trusted base caching, reporting, and baseline persistence to workflow_run. Authorize measured runs through maintainer-triggered reruns and update tests and documentation.
@bschnurr
Bill Schnurr (bschnurr) enabled auto-merge (squash) August 28, 2026 20:20
Export PNG previews alongside SVG dashboard charts and improve the single-baseline layout until additional hosted runs accumulate.
Run the repository's pinned Prettier formatter after notebook dashboard generation and commit the formatted HTML output.
@rchiodo

Copy link
Copy Markdown
Collaborator

🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR.

],
})
const commit = await github.rest.git.createCommit({
owner: context.repo.owner,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue · Please address or respond

Historical identity uses only result.date, so distinct base commits measured on the same UTC day target the same file and cannot both be retained. Include the source SHA or timestamp in the filename and add coverage for two same-day revisions.

[verified]


permissions:
contents: read
issues: read

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning · Non-blocking recommendation

Does issues: read authorize github.rest.pulls.get here? This workflow does not grant pull-requests: read, so requested reruns may receive a 403 while checking the PR. Add that permission or use an endpoint covered by the granted permissions.

issue_number: issueNumber,
name: 'benchmark-requested',
})
} catch (error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning · Non-blocking recommendation

Cleanup for an older report unconditionally removes the PR-wide request label, so a second /benchmark request made while that report is finishing can lose authorization before its rerun checks the label. Bind cleanup to a request/run identity or remove the label before rerunning, and test overlapping requests.

@rchiodo

Copy link
Copy Markdown
Collaborator

Result: ⚠️ partially-verified

Verification details

Verification: Isolated verification observed failures that were not classified as caused by this PR: Module-style benchmark test invocation, Benchmark unittest suite.

Summary: Five new comparator/profile tests passed. The 52-test benchmark suite ran 50 tests successfully, but two workflow tests errored because the container lacks Node.js. Verification is therefore partial; no PR-caused assertion failure was observed.

Test runs: 1 passed, 2 failed

  • Failed | unrelated to this PR | Module-style benchmark test invocation | python -m unittest build.benchmark.test_compare_benchmarks
  • Failed | unrelated to this PR | Benchmark unittest suite | python -m unittest discover -s build/benchmark -p "test_*.py"
  • Passed | New comparator and profile tests | python -m unittest test_compare_benchmarks.CompareBenchmarksTest.test_rejects_unexpected_source_revision test_compare_benchmarks.CompareBenchmarksTest.test_report_identifies_compared_revisions test_compare_benchmarks.CompareBenchmarksTest.test_allows_successful_benchmark_profile_change test_compare_benchmarks.CompareBenchmarksTest.test_incompatible_results_still_require_successful_measurements test_compare_benchmarks.CompareBenchmarksTest.test_pr_workflow_uses_matching_base_and_candidate_profiles
Module-style benchmark test invocation diagnostic output
ModuleNotFoundError: No module named 'build.benchmark'
Ran 1 test in 0.000s
FAILED (errors=1)
Benchmark unittest suite diagnostic output
Two workflow tests errored in _load_yaml with FileNotFoundError: [Errno 2] No such file or directory: 'node'.
Ran 52 tests in 0.057s
FAILED (errors=2)

@rchiodo Rich Chiodo (rchiodo) added the review-auto:changes-requested Automated review: posted blocking findings to address. label Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-auto:changes-requested Automated review: posted blocking findings to address.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants